blobcacheinfo,test: blobs must be resued when pushing across registry #5153
Conversation
|
@containers/buildah-maintainers @mtrmac PTAL |
|
Tests are green. |
tests/blobcache.bats
Outdated
|
|
||
| # Clear local image and c/image's blob-info-cache | ||
| run_buildah rmi --all -f | ||
| run rm /var/lib/containers/cache/blob-info-cache-v1.sqlite |
There was a problem hiding this comment.
The result of this run() isn't checked, and I'd be surprised if it succeeds for non-root users.
There was a problem hiding this comment.
@nalind New commit addresses this comment.
flouthoc
force-pushed
the
bump-c-image
branch
from
a42777c
to
2f35c96
Compare
tests/blobcache.bats
Outdated
| @test "blobcache: blobs must be reused when pushing across registry" { | ||
| start_registry | ||
| run_buildah login --tls-verify=false --authfile ${TEST_SCRATCH_DIR}/test.auth --username testuser --password testpassword localhost:${REGISTRY_PORT} | ||
| run_buildah pull registry.fedoraproject.org/fedora-minimal |
There was a problem hiding this comment.
An ignorant question, is there some mechanism to avoid reaching out to external registries that could be used here? I can see _prefetch, but that seeds c/storage, not a registry, so that seems not directly applicable. Or maybe something like that already works transparently?
Maybe this could, at least, only reach to r.fp.org once:
skopeo copy docker://…fp.org dir:original
buildah pull dir:original
… tag … push
# clean
buildah pull dir:original
… tag … push(I didn’t actually try running the above.)
There was a problem hiding this comment.
This SGTM i'll try this.
There was a problem hiding this comment.
@mtrmac Latest commit should address this, Could you PTAL again.
flouthoc
force-pushed
the
bump-c-image
branch
from
2f35c96
to
fa21338
Compare
flouthoc
force-pushed
the
bump-c-image
branch
from
fa21338
to
ea1543a
Compare
|
@mtrmac PTAL again |
There was a problem hiding this comment.
The test operation LGTM.
(#5153 (comment) is back but I’ll fully defer to repo maintainers on whether it is worth worrying about at all.)
flouthoc
force-pushed
the
bump-c-image
branch
from
ea1543a
to
f6c241e
Compare
@mtrmac Does latest commit resolves comment #5153 (comment) |
It seems we try to reuse blobs only for the specified registry, however we can have valid known compressed digests across registry as well following pr attempts to use that by doing following steps. * `CandidateLocations2` now processes all known blobs and appends them to returned candidates at the lowest priority. As a result when `TryReusingBlob` tries to process these candidates and if the blobs filtered by the `Opaque` set by the `transport` fail to match then attempt is made against all known blobs (ones which do not belong to the current registry). * Increase the sample set of potential blob reuse to all known compressed digests , also involving the one which do not belong to current registry. * If a blob is found match it against the registry where we are attempting to push. If blob is already there consider it a `CACHE HIT!` and reply skipping blob, since its already there. ---- ```console $ skopeo copy docker://registry.fedoraproject.org/fedora-minimal docker://quay.io/fl/test:some-tag $ buildah pull registry.fedoraproject.org/fedora-minimal $ buildah tag registry.fedoraproject.org/fedora-minimal quay.io/fl/test $ buildah push quay.io/fl/test ``` ```console Getting image source signatures Copying blob a3497ca15bbf skipped: already exists Copying config f7e02de757 done Writing manifest to image destination Storing signatures ``` Testing: containers/image#1645 Signed-off-by: Aditya R <arajan@redhat.com>
flouthoc
force-pushed
the
bump-c-image
branch
from
f6c241e
to
3c61dfd
Compare
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: flouthoc, rhatdan The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
It seems we try to reuse blobs only for the specified registry, however we can have valid known compressed digests across registry as well following pr attempts to test that use-case where feature was implemented in containers/image#1645
How to manually test this ?
Testing: containers/image#1645
What type of PR is this?
What this PR does / why we need it:
How to verify it
Which issue(s) this PR fixes:
Special notes for your reviewer:
Does this PR introduce a user-facing change?